Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ability to generate test operations for original values in the first object (continuation) #228

Merged
merged 16 commits into from
Jul 25, 2019
Merged

Ability to generate test operations for original values in the first object (continuation) #228

merged 16 commits into from
Jul 25, 2019

Conversation

warpech
Copy link
Collaborator

@warpech warpech commented Jul 2, 2019

Continuation of #226

Changes that I made compared to the original PR:

  • was: extra boolean argument on "observe" and extra object argument on "generate" and - "compare"
  • is: extra boolean argument "generate" and "compare"
  • changed the word inversible to invertible. Please correct me if I'm wrong
  • added way more tests for test operation generation

Pending questions:

  1. https://github.com/immerjs/immer also registers inverse patches, but they do it in a separate patch, not within the same patch. Is the solution in this PR more or less useful?

Benchmark results:

Before this PR (bbd3002, npm run build && npm run bench-duplex)

add operation
 Ops/sec: 3021278 ±0.33% Ran 154893 times in 0.051 seconds.
remove operation
 Ops/sec: 2467208 ±0.45% Ran 127226 times in 0.052 seconds.
replace operation
 Ops/sec: 2854277 ±0.28% Ran 146362 times in 0.051 seconds.
move operation
 Ops/sec: 271583 ±0.37% Ran 15241 times in 0.056 seconds.
copy operation
 Ops/sec: 214417 ±0.45% Ran 18773 times in 0.088 seconds.
test operation
 Ops/sec: 1316006 ±0.31% Ran 67352 times in 0.051 seconds.
===================
generate operation
 Ops/sec: 1680155 ±0.32% Ran 87099 times in 0.052 seconds.
generate operation and re-apply
 Ops/sec: 1483583 ±0.20% Ran 76006 times in 0.051 seconds.
compare operation
 Ops/sec: 427630 ±0.36% Ran 22101 times in 0.052 seconds.
compare operation same but deep objects
 Ops/sec: 122387928 ±0.31% Ran 6230468 times in 0.051 seconds.

After this PR

add operation
 Ops/sec: 3033581 ±0.26% Ran 155154 times in 0.051 seconds.
remove operation
 Ops/sec: 2513818 ±0.33% Ran 129460 times in 0.051 seconds.
replace operation
 Ops/sec: 2804656 ±0.31% Ran 144192 times in 0.051 seconds.
move operation
 Ops/sec: 271968 ±0.38% Ran 15132 times in 0.056 seconds.
copy operation
 Ops/sec: 215846 ±0.35% Ran 19086 times in 0.088 seconds.
test operation
 Ops/sec: 1311638 ±0.33% Ran 67526 times in 0.051 seconds.
==================
generate operation
 Ops/sec: 1705131 ±0.32% Ran 87963 times in 0.052 seconds.
generate operation and re-apply
 Ops/sec: 1452317 ±1.99% Ran 75998 times in 0.052 seconds.
compare operation
 Ops/sec: 422706 ±0.52% Ran 21843 times in 0.052 seconds.
compare operation same but deep objects
 Ops/sec: 115692461 ±0.71% Ran 6021898 times in 0.052 seconds.
================== new benchmarks start here: ==================
generate operation, invertible = true
 Ops/sec: 1607667 ±0.27% Ran 82412 times in 0.051 seconds.
generate operation and re-apply, invertible = true
 Ops/sec: 1401189 ±0.42% Ran 73320 times in 0.052 seconds.
compare operation, invertible = true
 Ops/sec: 237997 ±0.27% Ran 12237 times in 0.051 seconds.
compare operation same but deep objects, invertible = true
 Ops/sec: 116096872 ±0.50% Ran 5978518 times in 0.051 seconds.

pytlesk4 and others added 14 commits May 5, 2019 22:40
@pytlesk4
add test operation when generating patches
746ae6b
@pytlesk4
add inversible options
27711f6
@pytlesk4
update dist
479a6e8
@pytlesk4
don't introduce breaking change to observe api
4107704
@pytlesk4
- clean up function args
09ca15e 
- add inversible property to IObserver
- update readme
@pytlesk4
delete yarn lock file
a47f39b
@pytlesk4
fix formating
446a1ef
@pytlesk4
fixed formatting for test
4c9f3ce
@pytlesk4
fix formating for validate spec
5860c92
@pytlesk4
remove formatting from readme
bbda854
@pytlesk4
add benchmarks for generating test operations
b210dd6
@pytlesk4
- export all operations
15d4e8e
@warpech
Merge branch 'master' into generate-test-ops
f078a4b
@warpech
change the API for generating test operations
1498c09 
was: extra boolean argument on "observe" and extra object argument on "generate" and "compare"
is: extra boolean argument "generate" and "compare"

also this commit adds way more tests for test operation generation
@warpech warpech force-pushed the generate-test-ops-warpech branch from 8ac3dc3 to 1498c09 Compare July 2, 2019 09:38
@warpech warpech requested a review from tomalec July 2, 2019 09:38
@warpech warpech mentioned this pull request Jul 2, 2019
Closed
@warpech
Copy link
Collaborator Author

warpech commented Jul 2, 2019

cc @pytlesk4

tomalec
tomalec requested changes Jul 2, 2019
View reviewed changes
src/duplex.ts Outdated Show resolved Hide resolved
test/spec/duplexSpec.js Outdated Show resolved Hide resolved
@warpech
Copy link
Collaborator Author

warpech commented Jul 9, 2019

@tomalec please re-review.

I have achieved performing the same test using invertible set as true and then false by creating a new helper functions: variantIt, insertIf and usage of spread operator. I think this is quite neat, similar to NUnit fixtures. I wonder if this is readable, though. Please let me know.

@tomalec
Copy link
Collaborator

tomalec commented Jul 10, 2019

I wonder if this is readable, though. Please let me know.

At least to me it is :)

tomalec
tomalec approved these changes Jul 10, 2019
View reviewed changes
Copy link
Collaborator

@tomalec tomalec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pytlesk4
Copy link
Contributor

@warpech thanks for taking this over the finish line!

@tomalec
Copy link
Collaborator

tomalec commented Jul 11, 2019

@pytlesk4 once again thanks for starting a great PR and sorry for being picky :)

@warpech warpech merged commit 8e59102 into master Jul 25, 2019
@warpech warpech deleted the generate-test-ops-warpech branch July 25, 2019 10:40
@warpech
Copy link
Collaborator Author

warpech commented Jul 25, 2019 •
edited
Loading

Released today as 2.2.0

@pytlesk4
Copy link
Contributor

@warpech can this get published to NPM?

@warpech
Copy link
Collaborator Author

warpech commented Aug 1, 2019

Done, thanks for pinging me!

@snyk-bot snyk-bot mentioned this pull request Mar 26, 2020
Closed
@snyk-bot snyk-bot mentioned this pull request Apr 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomalec tomalec tomalec approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@warpech @tomalec @pytlesk4